feat:API for verify the authorization response#344
feat:API for verify the authorization response#344shitrerohit wants to merge 4 commits intofeat/oidc-main-syncfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR extends OpenID4VC holder and verifier functionality to support DCQL proof request verification. It propagates authorization options through the resolution flow, introduces a new verification endpoint for handling DCQL authorization responses with session tracking, and updates result handling to store complete submission results. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/controllers/openid4vc/holder/holder.service.ts`:
- Around line 231-233: Don't mutate submissionResult.serverResponse in place or
overwrite its body; instead create and return a new object that copies needed
HTTP metadata (e.g., status, headers) from submissionResult.serverResponse (or
uses defaults) and preserves its original body, and attach the full submission
payload under a new key (e.g., submission or submissionResult). Concretely, stop
assigning submissionResult.serverResponse to result and then setting
result['body'] = submissionResult; instead build a fresh object that merges
metadata from serverResponse and sets a separate field for the submission
payload so serverResponse.body is not discarded and no library-owned object is
mutated.
In
`@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`:
- Around line 203-210: The parameter name verifydcqlProofRquest in method
verifyDcqlProofRequest is misspelled and inconsistently cased; rename it to
verifyDCQLProofRequest (or verifyDcqlProofRequest -> verifyDCQLProofRequest)
throughout the service method signature and all call sites (including the
corresponding controller) and update the spread usage in
verifier.verifyAuthorizationResponse({ ...verifyDCQLProofRequest }) to use the
corrected identifier so types (OpenId4VCDCQLVerificationSessionRecord) and
naming are consistent.
- Line 208: Replace the incorrect call to
verifier.verifyAuthorizationResponse(...verifydcqlProofRquest) with
verifier.getVerifiedAuthorizationResponse(verificationSessionId); specifically,
stop spreading the verifydcqlProofRquest object and instead pass only the
verificationSessionId (the same id used earlier on line 118 usage), update the
surrounding code to accept the returned verified response type (remove the as
any cast if possible) and rename the local variable if needed to reflect that it
contains the verified authorization response.
🧹 Nitpick comments (3)
src/controllers/openid4vc/holder/holder.service.ts (1)
217-223: No-op try-catch — remove or add meaningful handling.The
catch (error) { throw error }block does nothing; it re-throws the same error. Either remove the try-catch entirely or add logging/wrapping before re-throwing.♻️ Proposed fix
- let dcqlCredentials - try { - dcqlCredentials = await agentReq.agent.modules.openid4vc.holder.selectCredentialsForDcqlRequest( - resolved.dcql.queryResult, - ) - } catch (error) { - throw error - } + const dcqlCredentials = await agentReq.agent.modules.openid4vc.holder.selectCredentialsForDcqlRequest( + resolved.dcql.queryResult, + )src/controllers/openid4vc/verifier-sessions/verification-sessions.Controller.ts (2)
90-103: Endpoint looks consistent with existing patterns; fix the parameter typo here too.The new endpoint follows the established try/catch +
ErrorHandlingService.handlepattern. Apply the same parameter rename (verifydcqlProofRquest→verifyDcqlProofRequest) as noted in the service file.✏️ Proposed rename
public async verifyDcqlProofRequest( `@Request`() request: Req, - `@Body`() verifydcqlProofRquest: OpenId4VCDCQLVerificationSessionRecord, + `@Body`() verifyDcqlProofRequest: OpenId4VCDCQLVerificationSessionRecord, ) { try { - return await this.verificationSessionService.verifyDcqlProofRequest(request, verifydcqlProofRquest) + return await this.verificationSessionService.verifyDcqlProofRequest(request, verifyDcqlProofRequest) } catch (error) { throw ErrorHandlingService.handle(error) } }
90-92: Consider adding a JSDoc note about expectedresponseModevalues.The PR description mentions this endpoint handles
dc_apianddc_api.jwtresponse modes, but the doc comment doesn't clarify this constraint. A brief note would help API consumers understand when to use this endpoint versus the existing flow.
src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts
Outdated
Show resolved
Hide resolved
| if (!verifier) { | ||
| throw new Error('OID4VC verifier module not initialized') | ||
| } | ||
| const result = (await verifier.verifyAuthorizationResponse({ ...verifydcqlProofRquest })) as any |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and examine the context around line 208
cat -n src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts | sed -n '200,220p'Repository: credebl/agent-controller
Length of output: 566
🏁 Script executed:
# Search for verifyAuthorizationResponse definition and usage
rg -n "verifyAuthorizationResponse" --type=ts -B3 -A5Repository: credebl/agent-controller
Length of output: 850
🏁 Script executed:
# Search for the verifydcqlProofRquest variable definition (note: possible typo)
rg -n "verifydcqlProofRquest|verifydcqlProofRequest" --type=ts -B3 -A3Repository: credebl/agent-controller
Length of output: 2672
🏁 Script executed:
# Search for OpenId4VCDCQLVerificationSessionRecord type definition
rg -n "OpenId4VCDCQLVerificationSessionRecord" --type=ts -B2 -A10Repository: credebl/agent-controller
Length of output: 6348
🏁 Script executed:
# Search for verifyAuthorizationResponse in the codebase or dependencies
rg -n "verifyAuthorizationResponse" --type=ts -B5 -A10 | head -100Repository: credebl/agent-controller
Length of output: 1193
🏁 Script executed:
# Find the type definition files for openid4vc verifier
fd -e "d.ts" -e "ts" | xargs grep -l "verifyAuthorizationResponse" 2>/dev/null | head -10Repository: credebl/agent-controller
Length of output: 144
🏁 Script executed:
# Check if there's a node_modules and search in credo-ts
find . -path "*/node_modules/@credo-ts/*/openid4vc*" -name "*.d.ts" 2>/dev/null | head -5Repository: credebl/agent-controller
Length of output: 50
🏁 Script executed:
# Look for verifyAuthorizationResponse in imports/type definitions
rg -n "verifyAuthorizationResponse" --type=ts --type=jsRepository: credebl/agent-controller
Length of output: 249
🌐 Web query:
credo-ts openid4vc verifier verifyAuthorizationResponse API signature
💡 Result:
In Credo TS (@credo-ts/openid4vc), the Verifier flow typically does not expose a public verifyAuthorizationResponse(...) API on agent.modules.openId4VcVerifier.
Instead, Credo verifies the incoming Authorization Response as part of the verification session flow, and you retrieve the result afterwards:
- Listen for the verification session reaching
ResponseVerified - Then call:
const verified = await verifier.modules.openId4VcVerifier.getVerifiedAuthorizationResponse(
verificationSessionId: string
)This pattern is shown in Credo-based verifier examples/docs. (docs.cheqd.io)
Citations:
🏁 Script executed:
# Search for getVerifiedAuthorizationResponse to see if it's used elsewhere
rg -n "getVerifiedAuthorizationResponse" --type=tsRepository: credebl/agent-controller
Length of output: 748
🏁 Script executed:
# Check if verifyAuthorizationResponse is defined in this codebase
rg -n "verifyAuthorizationResponse\s*\(" --type=ts -B2 -A10Repository: credebl/agent-controller
Length of output: 748
🏁 Script executed:
# Search for response verification patterns in the codebase
rg -n "ResponseVerified|getVerified" --type=tsRepository: credebl/agent-controller
Length of output: 748
🏁 Script executed:
# Look for imports from credo-ts openid4vc to understand what's available
rg -n "from.*credo.*openid4vc|from.*@credo-ts" --type=tsRepository: credebl/agent-controller
Length of output: 11763
Use getVerifiedAuthorizationResponse(verificationSessionId) instead of verifyAuthorizationResponse.
The method verifyAuthorizationResponse is not exposed by the Credo TS OpenID4VC verifier API. The correct method to retrieve verified authorization responses is getVerifiedAuthorizationResponse, which accepts only the verificationSessionId parameter (see line 118 in the same file for the correct usage pattern). The current code will fail at runtime.
🤖 Prompt for AI Agents
In `@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`
at line 208, Replace the incorrect call to
verifier.verifyAuthorizationResponse(...verifydcqlProofRquest) with
verifier.getVerifiedAuthorizationResponse(verificationSessionId); specifically,
stop spreading the verifydcqlProofRquest object and instead pass only the
verificationSessionId (the same id used earlier on line 118 usage), update the
surrounding code to accept the returned verified response type (remove the as
any cast if possible) and rename the local variable if needed to reflect that it
contains the verified authorization response.
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: shitrerohit <rohit.shitre@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
shitrerohit
force-pushed
the
feat/oid4vc-verify-authorization-response
branch
from
6748c94 to
b130bee
Compare
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts (1)
14-23:⚠️ Potential issue | 🔴 CriticalDuplicate imports will cause compilation error.
ClientIdPrefixandOpenId4VcIssuerX5cOptionsare imported twice in the same import block (lines 15/20 and 17/21 respectively). This will fail TypeScript compilation.🔧 Proposed fix to remove duplicates
import { ClientIdPrefix, CreateAuthorizationRequest, OpenId4VcIssuerX5cOptions, OpenId4VCDCQLVerificationSessionRecord, OpenId4VcIssuerX5c, - ClientIdPrefix, - OpenId4VcIssuerX5cOptions, ResponseModeEnum, } from '../types/verifier.types'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts` around lines 14 - 23, The import block in verification-sessions.service.ts contains duplicate symbols (ClientIdPrefix and OpenId4VcIssuerX5cOptions) which will break TypeScript compilation; edit the import list where ClientIdPrefix and OpenId4VcIssuerX5cOptions are listed and remove the duplicate occurrences so each symbol is imported only once (keep one entry for ClientIdPrefix and one for OpenId4VcIssuerX5cOptions alongside the other imports such as CreateAuthorizationRequest, OpenId4VCDCQLVerificationSessionRecord, OpenId4VcIssuerX5c, and ResponseModeEnum).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`:
- Around line 14-23: The import block in verification-sessions.service.ts
contains duplicate symbols (ClientIdPrefix and OpenId4VcIssuerX5cOptions) which
will break TypeScript compilation; edit the import list where ClientIdPrefix and
OpenId4VcIssuerX5cOptions are listed and remove the duplicate occurrences so
each symbol is imported only once (keep one entry for ClientIdPrefix and one for
OpenId4VcIssuerX5cOptions alongside the other imports such as
CreateAuthorizationRequest, OpenId4VCDCQLVerificationSessionRecord,
OpenId4VcIssuerX5c, and ResponseModeEnum).
---
Duplicate comments:
In
`@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`:
- Line 216: The code incorrectly calls verifier.verifyAuthorizationResponse(...)
which doesn't exist; replace this by waiting until the verification session
reaches the ResponseVerified state and then call
verifier.getVerifiedAuthorizationResponse(verificationSessionId) (the same
pattern used elsewhere in this file), passing the verificationSessionId and
handling/typing the returned result appropriately; remove the invalid
verifyAuthorizationResponse call and adapt any downstream logic to use the
object returned by getVerifiedAuthorizationResponse instead.
- Line 210: Rename the misspelled parameter verifydcqlProofRquest to
verifyDcqlProofRequest (camelCase) in the function/method declaration that
accepts OpenId4VCDCQLVerificationSessionRecord, and update every use site inside
the function (references, destructuring, JSDoc, and any callers) to the new
identifier; ensure exports, type annotations and tests that reference
verifydcqlProofRquest are updated to the new name so compilation and imports
stay consistent.
2 participants
What?
API for verify authorization response
How?
Created a new route to verify the authorization response received from a proof request when the response mode is
dc_apiordc_api.jwt.Summary by CodeRabbit
New Features
Bug Fixes